@dotnet-policy-service agree

@alexeyzimarev , I'm getting a build error for HttpUtility.ParseQueryString for net472, but heck if I can understand why...

@alexeyzimarev , ok, I've started this in the general direction you mentioned. Still more changes to come. Let me know what you think so far. Should we have a Options.RedirectOptions property that determines the behavior of forcing the method to GET for instance?

@alexeyzimarev , I've updated the branch for the interceptor changes. However, should After Request be called once for the last request in the redirect chain or repeatedly for each redirect?

Yeah, I think so. Interceptors don't care about how the request is done. It might change in the future if we decide to add OnRetry and OnRedirect, but with the current API I think you're right, just call it when it's all done.

@alexeyzimarev , I added ForwardHeaders/ForwardAuthorization/ForwardCookies tests... Should ForwardAuthorization support the Authenticator's parameter name instead of just KnownHeaders.Authorization?

@alexeyzimarev, I've added ForwardQuery, ForwardFragmnet, MaxRedirect, and FollowRedirectsToInsecure related tests.
This leaves ForwardBody (support/test), ForceForwardBody (test), AllowRedirectMethodStatusCodeToAlterVerb (test), and custom redirection status code additions for redirect detection (support and tests).

However, I wonder if the ForceForwardBody support is the best way to achieve that goal... Please take a look and see if you have any feedback.

All of the current test explorer tests pass, btw.

Thank you, I will have a look this week.

No problem. I managed to get Visual Studio .Net code coverage analysis working after getting nowhere with Coverlet and Fine Code Coverage. Hopefully, I'll be adding the rest of the test cases (indicated by coverage and otherwise) before the week's end.

I see why the secure server failed on the GitHub check; I didn't notice I had to export the private key into the certificate when running the tests locally for some reason. That fix will be in the next set of test case additions.

@alexeyzimarev, that's the last set of changes I will make before we discuss ForwardBody vs ForceForwardBody. If we should only have, say, ForceForwardBody since RedirectMethod status code (303) returns true from the helper method to determine if we should force GET instead of the current method.

I don't anticipate making any other changes before you get a chance to give me some feedback.

The code coverage looks excellent. I noticed that sometimes some of the non-"TestServer.cs" related tests fail due to trying to dispose of an already disposed server. I don't know if this is related to the xunit nuget pkg update. I also don't know if moving to IAsyncLifetime like TestServer.cs would fix them.

Bah, I just had this thought:
Should we ALWAYS regenerate the Cookie header after a redirect for security reasons?
And if so, should we even have ForceCookie? I'm leaning towards no.

For non-RestSharp users, if a supplied HttpClientHandler sets a CookieContainer, the HttpClient code does this as near as I can unravel from the source code. (That code makes my head hurt....) The basic idea is that the System.Net.Http inner handler of the RedirectHandler passes doAuth = false to SendAsync overloads of various sorts.

However, RestSharp doesn't supply the CookieContainer to a HttpClient HttpClientHandler, so we have to do it ourselves.

I am not sure. I thought we should collect the response headers and pass them over to the next call. It might, indeed, create security concerns if the response contains things like authentication cookie.
I didn't look at HttpClient code to see how they handle redirects. Instead, I checked Refit and I thought we could/should do the same thing, or similar. I am wondering if you are making it too good? :)

Hah. I think removing ForceCookie, regenerating the cookies for every redirected request (for cross-domain security reasons), and preventing the verb change without supporting ForwardBody or ForceForwardBody nonsense would be good enough. There's a reason the CookieContainer has you supply the requesting Uri for the GetCookie method after all. Hrm.. I looked at Refit briefly (https://github.com/reactiveui/refit/) but didn't see the bits that did header propagation. Did I look in the wrong place?

We could spin up a slack restsharp instance and have a conference call to discuss in a little more detail if you want or something similar...

Maybe create a Discord server?

Maybe create a Discord server?

@alexeyzimarev , https://discord.com/invite/MrGRGUug (expires in 7 days)